Skip to content

Fix: match adk-python when synthesizing a missing OpenAPI operationId - #800

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/openapi-operation-id-synthesis-parity
Open

Fix: match adk-python when synthesizing a missing OpenAPI operationId#800
AmaadMartin wants to merge 2 commits into
mainfrom
fix/openapi-operation-id-synthesis-parity

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

n/a

  1. Or, if no issue exists, describe the change:

Problem: An OpenAPI spec may omit operationId. Both SDKs synthesize one, but they synthesize different strings, so the same spec produces different tool names in TypeScript and Python. adk-js builds the id as method-then-path and replaces each non-alphanumeric character with its own underscore; adk-python builds it as _to_snake_case(f"{path}_{method}"). A user who ports an agent, an eval set, or a tool-filter list between the SDKs finds the tool names do not match.

Solution: Add an internal snakeCase(text) to core/src/utils/case_utils.ts and use it to synthesize the id from path-then-method. The helper reproduces the output of _to_snake_case in adk-python (src/google/adk/tools/_gemini_schema_util.py), so the two SDKs agree character for character. An operation that declares an operationId keeps it verbatim; the if (!operation.operationId) guard is untouched. Parity wins over local convention here because the tool name is observable across the language boundary.

Synthesized names, before and after:

path method before after (= adk-python)
/test get get__test test_get
/users/{id} get get__users__id_ users_id_get
/userProfiles/{userId} get get__userProfiles__userId_ user_profiles_user_id_get
/pets/{petId}/photos delete delete__pets__petId__photos pets_pet_id_photos_delete
/v1/API-Keys put put__v1_API_Keys v1_api_keys_put
/ get get__ get

This is a behaviour change. It affects only specs that omit operationId on at least one operation; a spec that names every operation is unaffected, including both in-repo fixtures. For affected users the tool name changes, so a pinned toolFilter entry, an eval set, or a prompt that names the tool breaks. The old names are not a designed format — they are the artefact of a per-character replace, with doubled and trailing underscores — and they diverge from the Python behaviour, which is the point of the change. No exported signature changes: snakeCase is internal and is not added to index.ts or common.ts.

getParamName is deliberately not touched. operation_parser.ts:46 holds a second, cruder snake_case converter, and it is what turns the id into the final tool name. Routing it through snakeCase too would also rename the parameters and tools of specs that declare an operationId (HTTPResponseCode becomes http_response_code instead of h_t_t_p_response_code), which is a wider break than this PR scopes itself to. #690 makes that change on its own.

The helper has five steps, not six. adk-python's _to_snake_case collapses repeated underscores as its fifth step, but that step cannot fire: step 1 already collapses each run of non-alphanumerics to a single _, and _ is itself non-alphanumeric, while steps 2 and 3 only ever insert an underscore between two alphanumeric characters. I checked it as well as asserted it: over every string of length up to 4 from a 16-character alphabet (lowercase, uppercase, digits, underscore, -, /, {, }, ., %, space, newline, non-ASCII) plus 500k random strings of length up to 20 — 569,904 inputs — the five-step and six-step forms return the same result for every one. Test mutation 5 below pins the collapsing on step 1, where it actually happens.

Collision check. I listed the 690 open PRs on this fork and read the diff of every adjacent one (#774, #748, #750, #690, #645, #605, #601, #462, #436, #389). None changes the synthesis line. #690 overlaps: it adds the same snakeCase helper and edits the same spec-parser test. I did not stack on it because its base is feat/apihub-toolset-part2, a multi-PR stack, which would pull unrelated work into this diff. The two helpers differ only by the dead step described above and return identical output, so whichever lands second should keep this five-step body. The expected names in this PR hold both before and after #690's getParamName change.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added: a snakeCase table in core/test/utils/case_utils_test.ts (11 rows, each row the output of adk-python's _to_snake_case for that input), and three cases in core/test/tools/openapi_tool/openapi_spec_parser_test.ts — multi-operation synthesis, two methods on one path, and a declared operationId that must survive.

npx vitest run --project unit:core core/test/utils/case_utils_test.ts \
  core/test/tools/openapi_tool/openapi_spec_parser_test.ts \
  core/test/tools/openapi_tool/openapi_toolset_test.ts \
  core/test/tools/openapi_tool/operation_parser_test.ts \
  core/test/tools/openapi_tool/openapi_toolset_integration_test.ts
# Test Files 5 passed (5) / Tests 51 passed (51)

npm run lint          # clean
npm run ts:check      # 292 errors, all pre-existing; 292 on main too, 0 new
npm run build --workspace core   # ok

openapi_toolset_integration_test.ts passes unedited. Both fixtures declare an operationId on every operation, so that suite is the evidence the change is inert for named specs.

Coverage of the changed source, measured over the five files above:
core/src/utils/case_utils.ts 100% statements, branches, functions and lines.
openapi_spec_parser.ts 99% lines; the two uncovered lines are 179-180 in sanitizeSchemaTypes, which this PR does not touch.

Two existing assertions changed, because they encode the bug. openapi_spec_parser_test.ts:204 pinned get__users__id_ and openapi_toolset_test.ts:253 pinned get__test. Both now pin the adk-python value. The edits are one line each plus one comment; the tests are otherwise untouched. Every other test in the touched files is unchanged, and no test was deleted, skipped, or weakened.

Proof the tests can fail. I ran the tests against five separate mutations. Each one failed.

  1. Synthesis line reverted to the old method-then-path template — 4 tests failed:
    • expected 'get__users__id_' to be 'users_id_get'
    • expected [ 'get__userProfiles__userId_', …(2) ] to deeply equal [ 'user_profiles_user_id_get', …(2) ]
    • expected [ 'get__test', 'post__test' ] to deeply equal [ 'test_get', 'test_post' ]
    • expected 'get__test' to be 'test_get'
      The "should not rewrite a declared operationId" case passes under this mutation by design: it pins the branch the mutation does not reach.
  2. Trailing-underscore strip narrowed to /^_+/g: expected 'leading_and_trailing_' to be 'leading_and_trailing'.
  3. Acronym rule deleted: expected 'httpresponse_code' to be 'http_response_code'.
  4. camelCase rule deleted — 4 rows failed, e.g. expected 'camelcase' to be 'camel_case'.
  5. + dropped from the first pattern, so runs stop collapsing: expected 'multiple___underscores' to be 'multiple_underscores'.

Manual End-to-End (E2E) Tests:

Not applicable. The change is a string transform inside the spec parser, with no network, model, or filesystem access. To see it by hand, parse a spec whose operation omits operationId:

const parsed = new OpenApiSpecParser().parse({
  openapi: '3.0.0',
  info: {title: 'Test', version: '1.0'},
  paths: {'/users/{id}': {get: {responses: {}}}},
} as unknown as OpenAPIV3.Document);
// parsed[0].name === 'users_id_get'

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Amaad Martin added 2 commits August 8, 2026 03:39
An OpenAPI operation may omit operationId. adk-js synthesized
`${method}_${path}` with per-character punctuation replacement, while
adk-python synthesizes _to_snake_case(f"{path}_{method}"). The same spec
therefore produced different tool names in the two SDKs.

Add a shared snakeCase helper that mirrors _to_snake_case in
src/google/adk/tools/_gemini_schema_util.py, and use it for synthesis.
An operation that declares an operationId keeps it verbatim.
…eCase

Step 1 already collapses each run of non-alphanumerics to a single
underscore, and an underscore is itself non-alphanumeric. Steps 2 and 3
only insert an underscore between two alphanumeric characters. The
repeated-underscore pass could therefore never match. Output is
unchanged, verified over 569904 inputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant